Add a much more detailed suite of tests for arb#340
Add a much more detailed suite of tests for arb#340oscarbenjamin merged 3 commits intoflintlib:mainfrom
Conversation
src/flint/test/test_arb.py
Outdated
|
|
||
| from flint import arb, ctx | ||
|
|
||
| from scipy.special import erf, erfc |
There was a problem hiding this comment.
This has caused the tests to fail in CI but I think that the math module's versions of these functions should be fine.
|
On hashing currently an arb does not necessarily compare equal to itself: In [1]: from flint import arb
In [2]: a = arb(1, 0.0001)
In [3]: a
Out[3]: [1.000 +/- 1.01e-4]
In [4]: a == a
Out[4]: FalseI'm not sure about that behaviour but it does make hashing less useful. |
Maegereg
force-pushed
the
dasm/arb-tests
branch
from
eacb118 to
d41ca53
Compare
|
Looks like arb doesn't have an |
|
Arb uses |
| true_interval = arb(6, 1.5) # [4.5, 7.5] | ||
| assert true_interval in actual | ||
|
|
||
| all_tests = [ |
There was a problem hiding this comment.
There should be a test that all the tests in this file are in this list or otherwise someone will end up adding tests here without adding them to the list. There is a test function that does this in test_all.py which could be adapted to do check for this file as well.
There was a problem hiding this comment.
Ah, I was wondering how that was handled. Done.
Have y'all considered switching to a test framework like pytest rather than implementing all of this manually?
There was a problem hiding this comment.
You can use pytest as well. The idea is just that you can run
python -m flint.test
to test your installation without needing to install anything.
Usually for development I would run spin test which ultimately calls pytest --pyargs flint.
There was a problem hiding this comment.
Fair enough! I do think requiring pytest would make the process of writing tests easier (and given that you're aiming to add more tests that might be a worthwhile tradeoff), but I can see why the ability to check the installation easily is useful too.
There was a problem hiding this comment.
We could require pytest but I don't think it would particularly make writing tests easier. Either way you mostly just write a lot of functions called test_foo. The potential advantages of pytest are things like selecting particular tests or running tests in parallel but the test suite is fast enough not to need those. One thing pytest is not good at is doctests which make up a good fraction of the test suite.
There was a problem hiding this comment.
Sounds good. I think this should be in good shape - do you want me to resolve threads, or should I leave it to you?
Ah, got it, that all makes sense now. In an ideal world you'd really want to use |
|
Now that tests are passed I think this looks good so let's get it in. Thanks! |
Arbs are immutable so hashing makes sense. It obviously needs to match equality though so as it stands it would only be useful for exact arbs. |
2 participants
Adapted from Tumult Core's tests for our own arb wrapper. I've modified these to work with the more bare-bones test setup
python-flintuses sincepytestorunittestdon't seem to be idiomatic.This does require adding a dev dependency on scipy - let me know if that's a problem. It makes the
erf,erfc, anderfinvtests much easier to write, but it's not strictly necessary.Tumult's arb wrapper supports hashing, so we have tests for it. I am inclined to leave them in and add hashing in a future PR, but let me know if that's not a feature we want.